Skip to content

test(otel): cover replay and chained execution - #796

Open
zhongkechen wants to merge 18 commits into
mainfrom
test/python-otel-conformance-20261021
Open

zhongkechen wants to merge 18 commits into
mainfrom
test/python-otel-conformance-20261021

Conversation

@zhongkechen

@zhongkechen zhongkechen commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

Add OpenTelemetry conformance handlers for user-function and callback contexts (22–23), invocation retries (24), caught callback-failure replay (25), external callback completion across later replays (26), and a chained child that suspends and resumes (27). Both views exercise the public Core 3 / OTel 2 APIs.

Merge prerequisite: #868 remains required for the ordinary local Case 26 tests. Keep these failures as merge gates; do not skip, xfail, or suppress them. The Case 25 fixes (#865 and #870) are already included from main. Cases 25–26 remain active in both cloud suites and the local suite. Case 21 retains its local replay tests but is explicitly NOT_IMPLEMENTED in both cloud views.

Validated review comments

Comment Validation and change
CI prerequisites Still valid for #868. Case 25 now passes; Case 26 fails in both views without callback-update production. The same suite passes with #868 alone; #867/#869 are not prerequisites for these tests.
Probe rejection guards Valid. Removed both redundant guards. Existing local and shared cloud checks retain exact SDK trace, parent, and raw export-count assertions.
Workflow pin and obsolete inputs Valid. Pin the reusable workflow to 0eaf7341c0c7ff937e6a4bf6bda1682ff07e7502, validate a full SHA, and remove unused phase/delay_seconds controls and pass-through.
Assertions consumed by retries Valid. Record Case 26 export counts and Case 22 context observations inside callbacks, then assert on the pytest thread. The export count is still observed before the target step runs.
Cloud history evidence Valid. All four saved Case 25 cloud histories contain Error: {Payload: {}, Truncated: false}. README and test comments cite the run; metadata-only history remains unverified against AWS Lambda.
Stale dependency list Valid. Replace the broad list with the remaining #868 dependency and record the fixes already merged.

Design

  • No public SDK API, runtime behavior, or new configuration is introduced. The review changes remove four handler source lines and obsolete workflow controls.
  • Probes use ordinary OpenTelemetry spans without supplying or repairing their parents. Existing trace/parent/count assertions detect absent or incorrect SDK context; redundant preflight rejection only adds a durable-callback failure path.
  • Test callbacks record observations instead of raising assertions that the SDK can retry. Case 26 asserts exactly [("target", 1)] before checking history gates. Later-barrier and final raw-count assertions remain. Case 22 keeps synchronized sibling probes and checks both callback and invocation-host context restoration after execution.
  • Existing OTel branch tests use different handlers. Four test-only barriers remain necessary to force overlap in the actual Case 22 child, parallel, and map callbacks. They neither inject nor repair tracing context.
  • Case 23 retains its unrelated-helper-failure and unexpected-success checks. Both protect required scenario execution and have negative public-runner coverage.
  • Case 21 remains deferred because the shared history requirements order InvocationCompleted before WaitSucceeded, while the timer can finish first. The recorded cloud run observed the reverse order on a successful execution. Increasing the wait only reduces the race. Re-enable cloud coverage after the shared requirement accepts both orders while preserving replay assertions.

Workflow and cloud evidence

The reusable workflow is pinned and maintained by Dependabot. The test catalog defaults to the conformance repository's main, with conformance_test_ref retained for manual overrides. Both PR and manual runs execute invocation and execution suites. All 13 remaining caller inputs and 7 secrets match the pinned schema; its default case count is 27. Legacy long-running templates and local contract tests remain, but there are no launch/check controls in this caller.

The previous four-job cloud run reported 25 passed, Case 25 failed on ERROR span status, and Case 21 NOT_IMPLEMENTED in every job. Case 26 already passed in the cloud. The saved detailed Case 25 histories verify the empty-error shape against AWS Lambda. Metadata-only Error: {Truncated: true} is a local-runner expectation and remains unverified against the service. The updated revision's cloud qualification is pending; a green run still would not claim Case 21 coverage.

The additional Case 17 callback-error regression remains outside this PR. Its source is preserved in 6484e577; correct its detailed/metadata-only history expectations before restoring it. The existing Case 17 handler remains active. The Case 11 fix is already merged in #864.

Validation

On Python 3.13.5 and Hatch 1.17.0:

  • Baseline at 10b1b31: 91 passed, 2 failed. Both failures are Case 26, reported as shifted history IDs after an assertion triggered an SDK step retry.
  • Updated branch: 92 passed, 2 failed. Both now directly report target completion must reach the exporter before the observed step, with [("target", 0)] instead of [("target", 1)].
  • With fix: record callback terminal operation updates #868 production changes temporarily applied: 94 passed. Those changes were then restored and are not included in this PR; fix: record callback terminal operation updates #868 remains a separate prerequisite.
  • Command: hatch run test:all packages/aws-durable-execution-sdk-python-conformance-tests-otel/tests .github/scripts/tests/test_opentelemetry_conformance_workflow.py -qq --tb=short.
  • Workflow regressions: 2 failed / 5 passed against the old caller; all 7 pass after pinning and removing obsolete controls.
  • The same 10 public-runner context/parent tests pass before and after guard removal. Mutations that remove SDK parenting still cause 4 Case 22 failures and 2 Case 23 failures. A separate context-restoration mutation causes 4 direct pytest failures. All mutations were restored.
  • Conformance-package formatting/lint, workflow-test formatting/lint, commit-message lint, and whitespace checks pass. Independent review found no actionable issues.

Existing test expectation changes

  • Template/catalog contract tests (test_templates_map_every_otel_requirement, test_templates_declare_only_the_expected_functions, test_function_names_identify_the_requirement, test_makefile_builds_every_function, test_every_registered_handler_exists, test_chained_invoke_functions_point_at_their_target, test_handler_modules_are_exactly_the_expected_set, test_active_callback_and_retry_handlers_keep_their_mapping, test_deferred_requirements_are_explicit) include active cloud cases 22–27, Case 27 targets, and retained local-only Case 21. Only Case 21 is pending.
  • test_callback_probes_keep_their_actual_sdk_parent adds unexpected helper-failure/success cases while keeping successful-scenario parent/count assertions.
  • test_external_callback_completion_precedes_later_replays checks (value, export_count) outside the step; the observation point and required count remain unchanged.
  • test_user_function_probes_keep_sdk_context_across_resume checks recorded callback/host context identities outside SDK retry handling and requires the expected observation counts.
  • test_opentelemetry_conformance_caller_uses_current_workflow_contract expects a full commit SHA; a new manual-input test prevents reintroducing inactive controls.

Case 27 companion PRs: JS #1068 and Java #824.

@github-actions

This comment has been minimized.

@zhongkechen zhongkechen mentioned this pull request Oct 21, 2026
7 of 11 tasks
@zhongkechen
zhongkechen added this pull request to stack #800 October 21, 2026 19:32
@zhongkechen
zhongkechen removed this pull request from stack #800 October 22, 2026 17:25
@zhongkechen
zhongkechen added this pull request to stack #809 October 22, 2026 17:25
@zhongkechen
zhongkechen temporarily deployed to ai-pr-review-runtime October 22, 2026 17:28 — with GitHub Actions Inactive
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@zhongkechen
zhongkechen temporarily deployed to ai-pr-review-runtime October 22, 2026 21:03 — with GitHub Actions Inactive
@github-actions

This comment has been minimized.

@zhongkechen
zhongkechen deployed to ai-pr-review-runtime October 28, 2026 18:35 — with GitHub Actions Active
@github-actions

This comment has been minimized.

Enable cases 25 and 26 in both cloud views and the local test suite.
Keep their regressions failing until the prerequisite fixes merge.

Preserve the callback completion export-time assertion and both later
replay barriers. Use raw span observations instead of configurable
proof-file reporting, and document the intended empty-history contract.

Validation: 89 focused tests pass and four expected regression cases
fail. Formatting, lint, and the 63 template contract tests pass.
@zhongkechen
zhongkechen deployed to ai-pr-review-runtime October 28, 2026 18:47 — with GitHub Actions Active
@github-actions

This comment has been minimized.

@zhongkechen
zhongkechen deployed to ai-pr-review-runtime October 28, 2026 21:08 — with GitHub Actions Active
Comment on lines +112 to +114
assert len(targets) == 1, (
"target completion must reach the exporter before the observed step"
)

This comment was marked as outdated.

@github-actions

This comment has been minimized.

yaythomas
yaythomas previously approved these changes Oct 31, 2026

@yaythomas yaythomas left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

per description, ready to merge once Python SDK PR #865: correct terminal telemetry status and timing and Python SDK PR #868: record callback terminal operation updates land. The inline comments are suggestions.

@zhongkechen
zhongkechen deployed to ai-pr-review-runtime November 1, 2026 04:26 — with GitHub Actions Active
Comment on lines +112 to +114
assert len(targets) == 1, (
"target completion must reach the exporter before the observed step"
)

This comment was marked as outdated.

@github-actions

This comment has been minimized.

Comment on lines +154 to +156
assert calls == [("target", 1)], (
"target completion must reach the exporter before the observed step"
)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Codex AI review · Finding arf_v1_ysa6hd7vcryj2bv5pbshd64jxt

[P1] Land the callback-update prerequisite before merging

The checked-out base's Execution.complete_callback_success() still does not populate updated_operation_ids. Consequently, callback completion is treated as terminal replay and its hook is omitted before the observed step, leaving the recorded export count at zero. Both parameterizations therefore fail in ordinary CI through the root testpaths and hatch run test:cov. Moving the assertion outside the step improves diagnostics but does not resolve this blocker. Land #868 and rebase before merging, preserving these regression assertions.

To suppress repeats, reply /ai verdict deferred, /ai verdict false-positive, or /ai verdict out-of-scope on the first nonblank line. Add rationale on later lines. A new /ai verdict accepted allows future repeats again.

@github-actions

github-actions Bot commented Nov 1, 2026

Copy link
Copy Markdown
Contributor

Codex AI review

One merge blocker remains at packages/aws-durable-execution-sdk-python-conformance-tests-otel/tests/test_external_callback_completion_replay.py:154: the callback-update prerequisite is absent. Tests were not executed during this read-only review.

Reviewed commit 6c7fd05ef1e8ba12425f132b96ec8a2601c418d9. Workflow run

This branch was successfully deployed

1 active deployment
ai-pr-review-runtime — 6c7fd05e Deployed Nov 1, 2026 by zhongkechen via ai-pr-review / Codex review / Generate Codex review #2693
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants